fix(obs-studio): reject non-finite geometry in source transforms - #397
Conversation
validate_position accepted NaN coordinates into project JSON. size/crop converted NaN via int() with a cryptic error. Reject non-finite values up front, matching validate_range.
validate_position/size/crop were unused while add_source and transform_source accepted NaN into project JSON. Call the validators from those paths and cover the live transform entrypoint.
github-actions
Bot
added
the
existing-cli-fix
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 516177b974
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Validate and normalize a size dict.""" | ||
| w = int(size.get("width", 1920)) | ||
| h = int(size.get("height", 1080)) | ||
| width_raw = float(size.get("width", 1920)) |
There was a problem hiding this comment.
For finite integer dimensions above IEEE-754's exact range, this new intermediate float conversion silently changes the requested geometry before int restores it: for example, width=2**53 + 1 is stored as 2**53, whereas the previous implementation preserved the integer exactly. This affects API callers and CLI-parsed integer sizes, and the identical conversion in validate_crop corrupts large crop values as well; validate finiteness without routing already-integral inputs through float.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks. Geometry validation now keeps already-integral values as ints and only float-converts non-ints when checking finiteness, so values above 2**53 are no longer rounded away.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f631e526
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(value, int): | ||
| return value | ||
| try: | ||
| num = float(value) |
There was a problem hiding this comment.
When direct API callers supply size or crop values as decimal integer strings—which the previous int(value) implementation accepted exactly—this conversion silently rounds values above IEEE-754's exact range; for example, validate_size({"width": "9007199254740993", "height": 1}) now returns width 9007199254740992. Fresh evidence beyond the earlier integer fix is that accepted integer strings still take this non-int branch and pass through float; parse integer-form inputs exactly before performing the finiteness check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4df5c850
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return int(value) | ||
| except ValueError: | ||
| pass | ||
| return int(_finite_number(value, name)) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier large-integer fixes is that exact integer-valued Fraction and Decimal inputs still take this fallback through float: Fraction(2**53 + 1, 1) and Decimal(2**53 + 1) both become 9007199254740992, whereas the previous direct int(value) preserved them exactly. This silently corrupts size and crop geometry for API callers using standard numeric types other than built-in int; perform the integer conversion without first narrowing these finite values to a float.
Useful? React with 👍 / 👎.
| return int(value) | ||
| except ValueError: | ||
| pass | ||
| return int(_finite_number(value, name)) |
There was a problem hiding this comment.
When a string fails the exact int(value) branch, this fallback converts it to a float and then truncates it, so inputs such as "1.5" and "1e3" are now silently accepted as 1 and 1000. The previous validators rejected both strings, and direct API callers can therefore receive unintended size or crop geometry instead of an input error; only finite integer-form strings should proceed to integer normalization.
Useful? React with 👍 / 👎.
| rot = float(rotation) | ||
| if not math.isfinite(rot): | ||
| raise ValueError(f"Rotation must be a finite number, got {rot}") |
There was a problem hiding this comment.
When one transform supplies a valid position, size, or crop together with a non-finite rotation, this newly added error occurs only after the earlier fields have already been assigned to the source. The caller therefore receives ValueError despite a partially applied transform; in the CLI path, source_transform also snapshots before this call and can retain that mutation. Validate all requested components, including rotation, before assigning any of them.
Useful? React with 👍 / 👎.
…validation atomic
|
Addressed review feedback:
|
2 participants
Problem
add_source/transform_sourceacceptedNaNposition coordinates into project JSON. Size/crop hit crypticint(NaN)errors instead of a clear validation failure.Repro
Fix
Validate position/size/crop with
math.isfiniteand call those helpers from the live source paths (complements #396 which coversvalidate_rangeonly).